Deprecate extraQueryParameters #1001
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request deprecates the extraQueryParameters() method and its builder counterparts across all parameter classes in the MSAL4J SDK. The deprecation indicates that this feature is not recommended for production scenarios and will be removed in a future release, potentially to be replaced by a new API.
- Adds
@Deprecatedannotations to methods and builder methods - Adds consistent deprecation messages across all affected classes
- Updates the interface
IAcquireTokenParameterswhere the method is declared
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| IAcquireTokenParameters.java | Deprecates the extraQueryParameters() method in the interface |
| UserNamePasswordParameters.java | Deprecates getter and builder methods for extra query parameters |
| SilentParameters.java | Deprecates getter and builder methods for extra query parameters |
| RefreshTokenParameters.java | Deprecates getter and builder methods for extra query parameters |
| OnBehalfOfParameters.java | Deprecates getter and builder methods for extra query parameters |
| InteractiveRequestParameters.java | Deprecates getter and builder methods for extra query parameters |
| IntegratedWindowsAuthenticationParameters.java | Deprecates getter and builder methods for extra query parameters |
| DeviceCodeFlowParameters.java | Deprecates getter and builder methods for extra query parameters |
| ClientCredentialParameters.java | Deprecates getter and builder methods for extra query parameters |
| AuthorizationRequestUrlParameters.java | Deprecates getter and builder methods for extra query parameters |
| AuthorizationCodeParameters.java | Deprecates getter and builder methods for extra query parameters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * @deprecated Not recommended for production scenarios. It will be removed in a future release, and the behavior may be replaced by a new API. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
Should this message be accompanied by the new API? I feel the message is a bit confusing if I am already using this API in production.
There was a problem hiding this comment.
agree, should we mimic MSAL .NET PR here as well?
There was a problem hiding this comment.
For all the other MSALs we decided to just deprecate the existing API, and not make all the other cache key changes like .NET
Long story short, for .NET we knew there were many customers using a lot of first-party ESTS features for testing (like getting already-expired tokens, simulating an outage, etc.). But for the other languages, the plan is more-or-less to deprecate the existing API and add the new behavior only if a customer specifically asks for it one day.
…ypes in msal-node (#8136) Removes the extraQueryParameters/extraParameters API according to KR [3310905](https://identitydivision.visualstudio.com/Engineering/_workitems/edit/3310905), similar to what is being done in MSAL.NET and MSAL Java: AzureAD/microsoft-authentication-library-for-dotnet#5536 AzureAD/microsoft-authentication-library-for-java#1001 This API was meant for niche scenarios which the library did not explicitly cover, and extensive usage could lead to bad caching behavior: these extra query parameters could affect the contents and validity of tokens but were not used in our caching scheme, so if they changed between requests we could return tokens that were not valid for the latest set of parameters. Therefore we are deprecating the current API and, if there is a need for it, may add a new API in the future to perform similar behavior in a more robust way (like was done in MSAL .NET due to requests from first-party customers). --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Deprecates the extraQueryParameters API according to KR 3310905, similar to what was done in MSAL.NET: AzureAD/microsoft-authentication-library-for-dotnet#5536
This API was meant for niche scenarios which the library did not explicitly cover, and not really meant for production. It can affect the contents and validity of tokens but were not used in our caching scheme which led to bad caching behavior: if the extra query parameters changed between requests we could return tokens that were not valid for the latest set of parameters.
Therefore we are deprecating the current API and, if there is a need for it, may add a new API in the future to perform similar behavior in a more robust way.